Skip to content

feat: add polling to BackendCapabilitiesBanner, persist OnboardingChe… - #1610

Merged
ritik4ever merged 1 commit into
ritik4ever:mainfrom
madhavi-0701:feat/frontend-enhancements
Jul 30, 2026
Merged

feat: add polling to BackendCapabilitiesBanner, persist OnboardingChe…#1610
ritik4ever merged 1 commit into
ritik4ever:mainfrom
madhavi-0701:feat/frontend-enhancements

Conversation

@madhavi-0701

@madhavi-0701 madhavi-0701 commented Jul 30, 2026

Copy link
Copy Markdown
Contributor

closes #1457
closes #1458
closes #1459
closes #1460

Summary by CodeRabbit

  • New Features
    • Added correlation detail modals from heatmap cells, including trend comparisons and loading states.
    • Onboarding progress now persists across sessions and includes a reset option.
    • Added an interactive drift breakdown showing per-asset contributions and total drift.
  • Improvements
    • Readiness updates poll less frequently when the browser tab is hidden, reducing background activity.
    • Capability notifications now remain stable while readiness data refreshes.

…cklist, add modal to CorrelationHeatmap, add breakdown tooltip to DriftGaugeGrid
@vercel

vercel Bot commented Jul 30, 2026

Copy link
Copy Markdown

Someone is attempting to deploy a commit to the ritik4ever's projects Team on Vercel.

A member of the Team first needs to authorize it.

@drips-wave

drips-wave Bot commented Jul 30, 2026

Copy link
Copy Markdown

@madhavi-0701 Great news! 🎉 Based on an automated assessment of this PR, the linked Wave issue(s) no longer count against your application limits.

You can now already apply to more issues while waiting for a review of this PR. Keep up the great work! 🚀

Learn more about application limits

@coderabbitai

coderabbitai Bot commented Jul 30, 2026

Copy link
Copy Markdown
Contributor

Review Change Stack

📝 Walkthrough

Walkthrough

Frontend updates add readiness polling and banner stabilization, correlation heatmap drill-downs, per-asset drift breakdowns, and local-storage persistence with reset support for onboarding progress. Tests were updated for the new query context, interactions, persistence, and polling behavior.

Changes

Backend readiness banner

Layer / File(s) Summary
Visibility-aware readiness polling
frontend/src/hooks/queries/useReadinessQuery.ts
Readiness polling uses a slower interval when the document is hidden.
Hook-driven capability banner
frontend/src/components/BackendCapabilitiesBanner.tsx, frontend/src/App.tsx
The banner reads readiness state internally, stabilizes notice changes, and is wired with only its positioning prop.
Banner polling and rendering tests
frontend/src/components/BackendCapabilitiesBanner.test.tsx
Tests cover readiness errors, notices, links, styling, and updated poll results.

Correlation heatmap drill-down

Layer / File(s) Summary
Asset-pair trend computation and modal
frontend/src/components/CorrelationHeatmap.tsx, frontend/src/components/ui/Modal.tsx
Heatmap cells select asset pairs, fetch daily prices, compute correlation trends, and display details in a modal.
Query-backed heatmap tests
frontend/src/components/CorrelationHeatmap.test.tsx
Tests provide React Query context and verify cell-click modal behavior alongside existing heatmap interactions.

Drift breakdown

Layer / File(s) Summary
Gauge rendering structure
frontend/src/components/DriftGauge.tsx
Gauge rendering and accessibility output are retained while arc calculations and component structure are simplified.
Per-asset breakdown interaction
frontend/src/components/DriftGauge.tsx, frontend/src/components/DriftGauge.test.tsx
The grid sorts assets by absolute drift and displays contribution percentages, totals, and interactive tooltip coverage.

Onboarding progress persistence

Layer / File(s) Summary
Persisted checklist state
frontend/src/components/OnboardingChecklist.tsx
Completed step IDs are loaded, reconciled with live statuses, persisted, and used to derive completion state.
Reset control and persistence tests
frontend/src/components/OnboardingChecklist.tsx, frontend/src/components/OnboardingChecklist.test.tsx
A reset button clears progress, with tests covering remount persistence and reset behavior.

Estimated code review effort: 4 (Complex) | ~45 minutes

Sequence Diagram(s)

sequenceDiagram
  participant User
  participant CorrelationHeatmap
  participant usePriceCandlestick
  participant Modal
  User->>CorrelationHeatmap: click distinct asset pair cell
  CorrelationHeatmap->>usePriceCandlestick: request daily close prices
  usePriceCandlestick-->>CorrelationHeatmap: return price series
  CorrelationHeatmap->>CorrelationHeatmap: calculate correlation trends
  CorrelationHeatmap-->>Modal: show selected-pair trend details
Loading
sequenceDiagram
  participant BackendCapabilitiesBanner
  participant useReadinessQuery
  participant CapabilitiesEndpoint
  BackendCapabilitiesBanner->>useReadinessQuery: subscribe to readiness state
  useReadinessQuery->>CapabilitiesEndpoint: poll using visibility-aware interval
  CapabilitiesEndpoint-->>useReadinessQuery: notices and errors
  useReadinessQuery-->>BackendCapabilitiesBanner: updated readiness result
  BackendCapabilitiesBanner->>BackendCapabilitiesBanner: update only when notice content changes
Loading

Possibly related PRs

Suggested reviewers: xqcxx

🚥 Pre-merge checks | ✅ 3 | ❌ 2

❌ Failed checks (2 warnings)

Check name Status Explanation Resolution
Description check ⚠️ Warning The description only links issues and omits the required change summary and most template sections. Add a brief summary, the change type, and the remaining checklist/template sections, or clearly state why they are omitted.
Docstring Coverage ⚠️ Warning Docstring coverage is 6.25% which is insufficient. The required threshold is 80.00%. Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (3 passed)
Check name Status Explanation
Title check ✅ Passed The title is relevant to the PR, but it is truncated and only names part of the overall change set.
Linked Issues check ✅ Passed The summarized code changes address all four linked feature requests and their stated acceptance criteria.
Out of Scope Changes check ✅ Passed The changes are focused on the four linked frontend features and related tests, with no clear unrelated additions.
✨ Finishing Touches 💡 1
🛠️ Fix failing CI checks 💡
  • Fix failing CI checks
🧪 Generate unit tests (beta)
  • Create PR with unit tests

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands.

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 11

🧹 Nitpick comments (3)
frontend/src/components/CorrelationHeatmap.test.tsx (1)

27-35: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick win

Shared QueryClient leaks cache between tests.

A single module-level client keeps candle query cache and refetch timers alive across all tests in the file, so the drill-down test's results can bleed into later tests. Create the client per test, or at least clear it in afterEach.

♻️ Minimal change
-afterEach(cleanup)
+afterEach(() => {
+  cleanup()
+  queryClient.clear()
+})
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@frontend/src/components/CorrelationHeatmap.test.tsx` around lines 27 - 35,
Update the test setup around the module-level queryClient and Wrapper so each
test receives a fresh QueryClient, or clear the shared client during afterEach
alongside cleanup. Ensure cached query data and refetch timers cannot persist
between tests.
frontend/src/components/CorrelationHeatmap.tsx (1)

271-309: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick win

Extract the repeated trend row.

The three rows differ only by label and value while duplicating the same pair of threshold ternaries six times. A small local row component (or a thresholdStyle(value) helper) removes the copy/paste and keeps thresholds in one place.

♻️ Sketch
const TREND_ROWS = [
  { label: 'Full period', key: 'fullPeriod' },
  { label: 'Recent half', key: 'recent' },
  { label: 'Older half', key: 'older' },
] as const

function trendStyle(value: number) {
  if (value > 0.3) return { color: '`#16a34a`', backgroundColor: '`#dcfce7`' }
  if (value < -0.3) return { color: '`#dc2626`', backgroundColor: '`#fee2e2`' }
  return { color: '`#6b7280`', backgroundColor: '`#f3f4f6`' }
}
<div className="space-y-3">
  {TREND_ROWS.map(({ label, key }) => (
    <div key={key} className="flex items-center justify-between p-3 rounded-lg bg-gray-50 dark:bg-gray-700/50">
      <span className="text-sm text-gray-600 dark:text-gray-400">{label}</span>
      <span className="text-sm font-semibold px-2 py-0.5 rounded" style={trendStyle(trendCorrelations[key])}>
        {trendCorrelations[key].toFixed(3)}
      </span>
    </div>
  ))}
</div>
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@frontend/src/components/CorrelationHeatmap.tsx` around lines 271 - 309,
Extract the duplicated trend rows in the trendCorrelations rendering into a
shared TREND_ROWS definition and mapped row markup, using a single
trendStyle(value) helper for the positive, negative, and neutral threshold
colors. Preserve the existing labels, keys, formatting, layout classes, and
threshold behavior.
frontend/src/components/BackendCapabilitiesBanner.tsx (1)

50-58: 🚀 Performance & Scalability | 🔵 Trivial | ⚡ Quick win

This ref does not suppress renders and is mutated during render.

When the readiness report changes but produces equivalent notices, BackendCapabilitiesBanner still re-renders because the hook result changed; updating lastNotices.current cannot prevent that. It also reads and writes ref.current during render, which React documents as unpredictable outside initialization. (uk.react.dev)

Move notice stabilization into useReadinessQuery using a selected, structurally shared notice payload, then render that value directly instead of maintaining a render-phase ref.

🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@frontend/src/components/BackendCapabilitiesBanner.tsx` around lines 50 - 58,
Move notice stabilization out of BackendCapabilitiesBanner and into
useReadinessQuery by selecting the notices payload with structural sharing, so
equivalent readiness updates reuse the previous notices reference. Remove the
lastNotices ref and render-phase comparison, and have BackendCapabilitiesBanner
consume the stabilized notices directly while preserving the existing
noticesEqual equivalence behavior.

Source: MCP tools

🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Inline comments:
In `@frontend/src/App.tsx`:
- Line 310: Unify readiness state in App by deriving showBackendBanner,
contentTopPad, and errorTop from the same useReadinessQuery result consumed by
BackendCapabilitiesBanner. Update the banner integration so both layout and
banner decisions use one readiness source, eliminating the separate
useReadinessReport polling path.

In `@frontend/src/components/BackendCapabilitiesBanner.test.tsx`:
- Around line 117-131: Update the test around BackendCapabilitiesBanner so it
keeps the same mounted component while simulating the poll change: retain the
render result, change mockQuery’s return value, and call rerender instead of
unmounting and rendering again. Preserve the assertions for the initial and
updated notices to verify an existing banner responds to changed notices.

In `@frontend/src/components/CorrelationHeatmap.test.tsx`:
- Around line 84-91: Update the drill-down test around CorrelationHeatmap to
mock usePriceCandlestick, or preload the relevant pair queries, so the trend
rendering and fetch behavior are deterministic. After clicking
correlation-cell-0-1, assert that the modal body contains both XLM and BTC in
addition to the existing title assertions, verifying the clicked asset pair
context.

In `@frontend/src/components/CorrelationHeatmap.tsx`:
- Around line 57-60: Update the return calculations in the loop within
CorrelationHeatmap so each zero/missing-value guard checks the previous price
used as the divisor (`pricesA[i - 1]` and `pricesB[i - 1]`), rather than the
current price at index i. Preserve the existing fallback of 0 while ensuring
denominators are validated before division.
- Around line 100-116: Update the trendCorrelations useMemo guard to return null
unless each candle dataset’s asset matches the currently selectedPair,
preventing placeholder candles from the previous pair from being used. Include
selectedPair in the memo dependency array while preserving the existing
correlation calculations for matching assets.
- Around line 310-312: Update the correlation trend rendering in
CorrelationHeatmap to use the isLoading and isError states from both
usePriceCandlestick calls, and handle datasets with fewer than 7 candles
explicitly. Show the loading placeholder only while either request is loading,
and render a clear failure or insufficient-history state instead of leaving the
loading message indefinitely.

In `@frontend/src/components/DriftGauge.test.tsx`:
- Around line 83-110: Update the test for DriftGaugeGrid’s title-hover tooltip
to assert the calculated contribution values in addition to ordering: verify XLM
displays a 42% contribution and the breakdown shows a 12.0% total, while
preserving the existing asset-order and drift assertions.

In `@frontend/src/components/DriftGauge.tsx`:
- Around line 254-265: Update the DriftGauge component to generate a stable
tooltip identifier with React useId, assign it to the conditional breakdown
element with role="tooltip", and add the matching aria-describedby reference to
the drift-gauge-heading element. Keep the association valid when showBreakdown
is false.
- Around line 230-240: Move the assets-empty check in DriftGaugeGrid below the
sortedAssets and totalDrift useMemo hooks so every render invokes hooks in the
same order. Preserve returning null for empty assets while keeping the existing
memoized calculations unchanged.

In `@frontend/src/components/OnboardingChecklist.tsx`:
- Around line 100-170: Update the liveStepStatuses synchronization in
OnboardingChecklist so persisted steps are added only when a live status
transitions from false to true, preventing unrelated status changes from
resurrecting steps cleared by Reset. Consolidate the initial-load merge and
ongoing sync into one effect, track previous live statuses, and include all
referenced state in dependencies without relying on a stale completedSteps
closure. Add a regression test covering reset followed by an unrelated
live-status change while another condition remains true.

In `@frontend/src/hooks/queries/useReadinessQuery.ts`:
- Around line 57-69: The useReadinessQuery polling interval does not reliably
adapt to tab visibility changes. Add SSR-safe React visibility state updated by
a visibilitychange listener, use that isVisible state in refetchInterval to
select POLL_MS or HIDDEN_POLL_MS, and enable refetchIntervalInBackground so
hidden-tab polling continues at the slower rate.

---

Nitpick comments:
In `@frontend/src/components/BackendCapabilitiesBanner.tsx`:
- Around line 50-58: Move notice stabilization out of BackendCapabilitiesBanner
and into useReadinessQuery by selecting the notices payload with structural
sharing, so equivalent readiness updates reuse the previous notices reference.
Remove the lastNotices ref and render-phase comparison, and have
BackendCapabilitiesBanner consume the stabilized notices directly while
preserving the existing noticesEqual equivalence behavior.

In `@frontend/src/components/CorrelationHeatmap.test.tsx`:
- Around line 27-35: Update the test setup around the module-level queryClient
and Wrapper so each test receives a fresh QueryClient, or clear the shared
client during afterEach alongside cleanup. Ensure cached query data and refetch
timers cannot persist between tests.

In `@frontend/src/components/CorrelationHeatmap.tsx`:
- Around line 271-309: Extract the duplicated trend rows in the
trendCorrelations rendering into a shared TREND_ROWS definition and mapped row
markup, using a single trendStyle(value) helper for the positive, negative, and
neutral threshold colors. Preserve the existing labels, keys, formatting, layout
classes, and threshold behavior.
🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

  • Push a commit to this branch (recommended)
  • Create a new PR with the fixes

ℹ️ Review info
⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro Plus

Run ID: 6d55b7ac-0bb8-4c91-9163-d127d4c1703a

📥 Commits

Reviewing files that changed from the base of the PR and between 310e8df and d0ce70b.

⛔ Files ignored due to path filters (1)
  • frontend/src/components/__snapshots__/DriftGauge.test.tsx.snap is excluded by !**/*.snap
📒 Files selected for processing (11)
  • frontend/src/App.tsx
  • frontend/src/components/BackendCapabilitiesBanner.test.tsx
  • frontend/src/components/BackendCapabilitiesBanner.tsx
  • frontend/src/components/CorrelationHeatmap.test.tsx
  • frontend/src/components/CorrelationHeatmap.tsx
  • frontend/src/components/DriftGauge.test.tsx
  • frontend/src/components/DriftGauge.tsx
  • frontend/src/components/OnboardingChecklist.test.tsx
  • frontend/src/components/OnboardingChecklist.tsx
  • frontend/src/components/ui/Modal.tsx
  • frontend/src/hooks/queries/useReadinessQuery.ts
💤 Files with no reviewable changes (1)
  • frontend/src/components/ui/Modal.tsx

Comment thread frontend/src/App.tsx
loading={readinessLoading}
belowRealtimeBar={false}
/>
<BackendCapabilitiesBanner belowRealtimeBar={false} />

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🎯 Functional Correctness | 🟠 Major | ⚡ Quick win

🧩 Analysis chain

🏁 Script executed:

#!/bin/bash
set -euo pipefail
rg -n -C 6 'useReadinessReport|useReadinessQuery|readinessKeys|fetchReadinessReport|setInterval' frontend/src

Repository: ritik4ever/stellar-portfolio-rebalancer

Length of output: 32422


🏁 Script executed:

#!/bin/bash
set -euo pipefail

echo "== App readiness/layout declarations and banner invocation =="
sed -n '45,68p' frontend/src/App.tsx
echo
sed -n '280,325p' frontend/src/App.tsx
echo
echo "== read-only hook/body evidence =="
sed -n '1,90p' frontend/src/hooks/queries/useReadinessQuery.ts
echo
sed -n '165,208p' frontend/src/hooks/useReadinessReport.ts
echo
echo "== Behavioral model from source text =="
python3 - <<'PY'
from pathlib import Path
app = Path('frontend/src/App.tsx').read_text()
banner = Path('frontend/src/components/BackendCapabilitiesBanner.tsx').read_text()
report = Path('frontend/src/hooks/useReadinessReport.ts').read_text()
query = Path('frontend/src/hooks/queries/useReadinessQuery.ts').read_text()
checks = {
    'App uses both readiness hooks': 'useReadinessReport' in app and 'useReadinessQuery' in banner and app.count('BackendCapabilitiesBanner') >= 1,
    'useReadinessQuery uses React Query interval/backoff': 'useQuery' in query and 'refetchInterval: () => document.visibilityState' in query,
    'useReadinessReport uses manual polling interval/backoff': 'window.setInterval' in report and 'visibilitychange' in report,
    'App derives showBackendBanner from useReadinessReport': 'showBackendBanner' in app and 'notices.some' in app,
    'layout top-padding class from showBackendBanner': 'pt-4' in app and 'contentTopPad' in app,
    'errorTop uses report from useReadinessReport': 'errorTop' in app and 'report' in app and 'useReadinessReport' in app,
}
for name, ok in checks.items():
    print(f'{name}: {ok}')
PY

Repository: ritik4ever/stellar-portfolio-rebalancer

Length of output: 8725


🏁 Script executed:

#!/bin/bash
set -euo pipefail

echo "== App relevant sections =="
sed -n '62,80p' frontend/src/App.tsx
sed -n '200,320p' frontend/src/App.tsx

echo
echo "== useReadinessReport refresh and any App/other use/useQueryClient references =="
rg -n -C 4 'function useReadinessReport|refresh|useQueryClient|invalidateQueries|reset|useReadinessReport\(|useReadinessQuery\(' frontend/src/App.tsx frontend/src/hooks/utils frontend/src/hooks/queries frontend/src/hooks use

Repository: ritik4ever/stellar-portfolio-rebalancer

Length of output: 39398


Use one readiness source for App layout and the banner.

App still derives showBackendBanner, contentTopPad, and errorTop from useReadinessReport, while BackendCapabilitiesBanner reads useReadinessQuery. Since refresh() is not passed into App, readiness can update in the banner before the page offset/error positioning catch up, and the two hooks also poll separately. Derive all readiness decisions from useReadinessQuery or pass the same hook result into the banner.

🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@frontend/src/App.tsx` at line 310, Unify readiness state in App by deriving
showBackendBanner, contentTopPad, and errorTop from the same useReadinessQuery
result consumed by BackendCapabilitiesBanner. Update the banner integration so
both layout and banner decisions use one readiness source, eliminating the
separate useReadinessReport polling path.

Comment on lines +117 to +131
it('updates banner after a simulated capability change on subsequent poll', () => {
mockReturn({ notices: [notice('database', 'limited', 'Initial issue.')] })
const { unmount } = render(<BackendCapabilitiesBanner />)
expect(screen.getByText(/Initial issue\./)).toBeInTheDocument()
unmount()

mockQuery.mockReturnValue({
notices: [notice('database', 'limited', 'Updated issue.')],
loadError: false,
loading: false,
report: null,
refresh: vi.fn(),
})
render(<BackendCapabilitiesBanner />)
expect(screen.getByText(/Updated issue\./)).toBeInTheDocument()

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win

Keep the component mounted when simulating a poll update.

Unmounting before changing the mock resets lastNotices, so this test only proves that a fresh mount renders the new notice. It would pass even if an existing banner ignored updated notices.

Update the mock return value and call rerender without unmounting.

Proposed test adjustment
-        const { unmount } = render(<BackendCapabilitiesBanner />)
+        const view = render(<BackendCapabilitiesBanner />)
         expect(screen.getByText(/Initial issue\./)).toBeInTheDocument()
-        unmount()

         mockQuery.mockReturnValue({
             notices: [notice('database', 'limited', 'Updated issue.')],
             loadError: false,
             loading: false,
             report: null,
             refresh: vi.fn(),
         })
-        render(<BackendCapabilitiesBanner />)
+        view.rerender(<BackendCapabilitiesBanner />)
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
it('updates banner after a simulated capability change on subsequent poll', () => {
mockReturn({ notices: [notice('database', 'limited', 'Initial issue.')] })
const { unmount } = render(<BackendCapabilitiesBanner />)
expect(screen.getByText(/Initial issue\./)).toBeInTheDocument()
unmount()
mockQuery.mockReturnValue({
notices: [notice('database', 'limited', 'Updated issue.')],
loadError: false,
loading: false,
report: null,
refresh: vi.fn(),
})
render(<BackendCapabilitiesBanner />)
expect(screen.getByText(/Updated issue\./)).toBeInTheDocument()
it('updates banner after a simulated capability change on subsequent poll', () => {
mockReturn({ notices: [notice('database', 'limited', 'Initial issue.')] })
const view = render(<BackendCapabilitiesBanner />)
expect(screen.getByText(/Initial issue\./)).toBeInTheDocument()
mockQuery.mockReturnValue({
notices: [notice('database', 'limited', 'Updated issue.')],
loadError: false,
loading: false,
report: null,
refresh: vi.fn(),
})
view.rerender(<BackendCapabilitiesBanner />)
expect(screen.getByText(/Updated issue\./)).toBeInTheDocument()
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@frontend/src/components/BackendCapabilitiesBanner.test.tsx` around lines 117
- 131, Update the test around BackendCapabilitiesBanner so it keeps the same
mounted component while simulating the poll change: retain the render result,
change mockQuery’s return value, and call rerender instead of unmounting and
rendering again. Preserve the assertions for the initial and updated notices to
verify an existing banner responds to changed notices.

Comment on lines +84 to +91
it('opens a drill-down modal when a cell is clicked with correct asset pair context', async () => {
render(<CorrelationHeatmap assets={assets} correlations={matrices} />, { wrapper: Wrapper })

fireEvent.click(screen.getByTestId('correlation-cell-0-1'))

expect(screen.getByText(/correlation detail/i)).toBeInTheDocument()
expect(screen.getByRole('heading', { name: /correlation detail/i })).toBeInTheDocument()
})

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick win

🧩 Analysis chain

🏁 Script executed:

#!/bin/bash
fd -H -t f 'vitest.config.*|vite.config.*' frontend --exec cat -n
fd -H -t f 'setup*.ts*' frontend/src --exec cat -n
rg -n --type=ts --type=tsx 'usePriceCandlestickQuery|PRICE_CHART' frontend/src -g '!**/*.test.*'

Repository: ritik4ever/stellar-portfolio-rebalancer

Length of output: 50396


🏁 Script executed:

#!/bin/bash
set -euo pipefail

echo "== locate heatmap tests and component =="
fd -H -t f 'CorrelationHeatmap' frontend

echo
echo "== test excerpt =="
file="$(fd -H -t f 'CorrelationHeatmap.test.tsx' frontend | head -n1 || true)"
if [ -n "$file" ]; then
  wc -l "$file"
  sed -n '1,140p' "$file" | cat -n
fi

echo
echo "== component outline relevant file =="
if [ -n "$file" ]; then
  comp="$(git rev-parse --show-toplevel)/frontend/src/components/CorrelationHeatmap.tsx"
  if [ -f "$comp" ]; then
    wc -l "$comp"
    ast-grep outline "$comp" --view expanded || true
    sed -n '1,260p' "$comp" | cat -n
  fi
fi

echo
echo "== hook usages =="
rg -n --type=ts --type=tsx 'usePriceCandlestick|QueryClient|refetchInterval|DataChart|data:' frontend/src -g '!**/*.test.*' --glob '!**/node_modules/**' --max-count 200

Repository: ritik4ever/stellar-portfolio-rebalancer

Length of output: 17764


🏁 Script executed:

#!/bin/bash
set -euo pipefail

echo "== remaining component render/modal section =="
file="$(git rev-parse --show-toplevel)/frontend/src/components/CorrelationHeatmap.tsx"
sed -n '260,330p' "$file" | cat -n

echo
echo "== hook file =="
hook="$(git rev-parse --show-toplevel)/frontend/src/hooks/queries/usePriceCandlestickQuery.ts" || true
if [ -f "$hook" ]; then
  wc -l "$hook"
  sed -n '1,220p' "$hook" | cat -n
else
  fd -H -t f 'usePriceCandlestickQuery' frontend
fi

echo
echo "== global fetch/api mocks in Vitest setup files =="
sed -n '1,180p' frontend/src/test/setup.ts | cat -n
rg -n --glob '!**/node_modules/**' 'vi\.stubGlobal\(|fetch|global.fetch|axios|XMLHttpRequest|msw|setupServer|http|api|usePriceCandlestickQuery|queryClient' frontend/src frontend/vitest.config.ts frontend/src/test/setup.ts

Repository: ritik4ever/stellar-portfolio-rebalancer

Length of output: 50395


Assert the clicked pair and mock the candlestick query.

The assertions only check the static modal title, so this would pass for the wrong XLM/BTC modal; assert that XLM and BTC appear in the modal body, and mock usePriceCandlestick (or preload the pair’s queries) so the trend path and its fetch are deterministic.

🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@frontend/src/components/CorrelationHeatmap.test.tsx` around lines 84 - 91,
Update the drill-down test around CorrelationHeatmap to mock
usePriceCandlestick, or preload the relevant pair queries, so the trend
rendering and fetch behavior are deterministic. After clicking
correlation-cell-0-1, assert that the modal body contains both XLM and BTC in
addition to the existing title assertions, verifying the clicked asset pair
context.

Comment on lines +57 to +60
for (let i = 1; i < pricesA.length; i++) {
returnsA.push(pricesA[i] > 0 ? (pricesA[i] - pricesA[i - 1]) / pricesA[i - 1] : 0)
returnsB.push(pricesB[i] > 0 ? (pricesB[i] - pricesB[i - 1]) / pricesB[i - 1] : 0)
}

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🎯 Functional Correctness | 🟠 Major | ⚡ Quick win

Zero-guard checks the wrong index.

The divisor is pricesA[i - 1], but the guard inspects pricesA[i]. With a zero/missing previous close the division yields Infinity, which poisons the means and variances so the whole series collapses to 0 (only saved from NaN by clampCorrelation). Guard the denominator instead.

🐛 Proposed fix
-  for (let i = 1; i < pricesA.length; i++) {
-    returnsA.push(pricesA[i] > 0 ? (pricesA[i] - pricesA[i - 1]) / pricesA[i - 1] : 0)
-    returnsB.push(pricesB[i] > 0 ? (pricesB[i] - pricesB[i - 1]) / pricesB[i - 1] : 0)
-  }
+  for (let i = 1; i < pricesA.length; i++) {
+    if (!(pricesA[i - 1] > 0) || !(pricesB[i - 1] > 0)) continue
+    returnsA.push((pricesA[i] - pricesA[i - 1]) / pricesA[i - 1])
+    returnsB.push((pricesB[i] - pricesB[i - 1]) / pricesB[i - 1])
+  }
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
for (let i = 1; i < pricesA.length; i++) {
returnsA.push(pricesA[i] > 0 ? (pricesA[i] - pricesA[i - 1]) / pricesA[i - 1] : 0)
returnsB.push(pricesB[i] > 0 ? (pricesB[i] - pricesB[i - 1]) / pricesB[i - 1] : 0)
}
for (let i = 1; i < pricesA.length; i++) {
if (!(pricesA[i - 1] > 0) || !(pricesB[i - 1] > 0)) continue
returnsA.push((pricesA[i] - pricesA[i - 1]) / pricesA[i - 1])
returnsB.push((pricesB[i] - pricesB[i - 1]) / pricesB[i - 1])
}
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@frontend/src/components/CorrelationHeatmap.tsx` around lines 57 - 60, Update
the return calculations in the loop within CorrelationHeatmap so each
zero/missing-value guard checks the previous price used as the divisor
(`pricesA[i - 1]` and `pricesB[i - 1]`), rather than the current price at index
i. Preserve the existing fallback of 0 while ensuring denominators are validated
before division.

Comment on lines +100 to +116
const trendCorrelations = useMemo(() => {
if (!candleDataA || !candleDataB) return null
const pricesA = candleDataA.candles.map((c) => c.close)
const pricesB = candleDataB.candles.map((c) => c.close)
if (pricesA.length < 7 || pricesB.length < 7) return null

const minLen = Math.min(pricesA.length, pricesB.length)
const alignedA = pricesA.slice(pricesA.length - minLen)
const alignedB = pricesB.slice(pricesB.length - minLen)

const fullPeriod = computePearsonCorrelation(alignedA, alignedB)
const mid = Math.floor(minLen / 2)
const recent = computePearsonCorrelation(alignedA.slice(mid), alignedB.slice(mid))
const older = computePearsonCorrelation(alignedA.slice(0, mid), alignedB.slice(0, mid))

return { fullPeriod, recent, older }
}, [candleDataA, candleDataB])

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win

Guard against stale candle data from the previously selected pair.

usePriceCandlestick uses placeholderData: (prev) => prev, so right after switching pairs the hooks can return the previous asset's candles while the new query resolves. Since the memo only depends on the data objects, the modal will render numbers computed for the old pair under the new pair's heading. Validate the returned asset against selectedPair (and include it in the deps).

🛡️ Proposed guard
   const trendCorrelations = useMemo(() => {
-    if (!candleDataA || !candleDataB) return null
+    if (!selectedPair || !candleDataA || !candleDataB) return null
+    if (
+      candleDataA.asset !== selectedPair.rowAsset ||
+      candleDataB.asset !== selectedPair.columnAsset
+    ) {
+      return null
+    }
     const pricesA = candleDataA.candles.map((c) => c.close)
@@
     return { fullPeriod, recent, older }
-  }, [candleDataA, candleDataB])
+  }, [candleDataA, candleDataB, selectedPair])
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
const trendCorrelations = useMemo(() => {
if (!candleDataA || !candleDataB) return null
const pricesA = candleDataA.candles.map((c) => c.close)
const pricesB = candleDataB.candles.map((c) => c.close)
if (pricesA.length < 7 || pricesB.length < 7) return null
const minLen = Math.min(pricesA.length, pricesB.length)
const alignedA = pricesA.slice(pricesA.length - minLen)
const alignedB = pricesB.slice(pricesB.length - minLen)
const fullPeriod = computePearsonCorrelation(alignedA, alignedB)
const mid = Math.floor(minLen / 2)
const recent = computePearsonCorrelation(alignedA.slice(mid), alignedB.slice(mid))
const older = computePearsonCorrelation(alignedA.slice(0, mid), alignedB.slice(0, mid))
return { fullPeriod, recent, older }
}, [candleDataA, candleDataB])
const trendCorrelations = useMemo(() => {
if (!selectedPair || !candleDataA || !candleDataB) return null
if (
candleDataA.asset !== selectedPair.rowAsset ||
candleDataB.asset !== selectedPair.columnAsset
) {
return null
}
const pricesA = candleDataA.candles.map((c) => c.close)
const pricesB = candleDataB.candles.map((c) => c.close)
if (pricesA.length < 7 || pricesB.length < 7) return null
const minLen = Math.min(pricesA.length, pricesB.length)
const alignedA = pricesA.slice(pricesA.length - minLen)
const alignedB = pricesB.slice(pricesB.length - minLen)
const fullPeriod = computePearsonCorrelation(alignedA, alignedB)
const mid = Math.floor(minLen / 2)
const recent = computePearsonCorrelation(alignedA.slice(mid), alignedB.slice(mid))
const older = computePearsonCorrelation(alignedA.slice(0, mid), alignedB.slice(0, mid))
return { fullPeriod, recent, older }
}, [candleDataA, candleDataB, selectedPair])
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@frontend/src/components/CorrelationHeatmap.tsx` around lines 100 - 116,
Update the trendCorrelations useMemo guard to return null unless each candle
dataset’s asset matches the currently selectedPair, preventing placeholder
candles from the previous pair from being used. Include selectedPair in the memo
dependency array while preserving the existing correlation calculations for
matching assets.

Comment on lines +83 to 110
it('shows per-asset breakdown tooltip on title hover sorted by largest drift first', () => {
render(<DriftGaugeGrid assets={sampleAssets} />)

const heading = screen.getByText('Allocation Drift')
fireEvent.mouseEnter(heading)

const tooltip = screen.getByRole('tooltip')
expect(tooltip).toHaveTextContent('Drift Breakdown')

const tooltipText = tooltip.textContent || ''
const xlmIndex = tooltipText.indexOf('XLM')
const btcIndex = tooltipText.indexOf('BTC')
const ethIndex = tooltipText.indexOf('ETH')
const usdcIndex = tooltipText.indexOf('USDC')

expect(xlmIndex).toBeGreaterThan(-1)
expect(btcIndex).toBeGreaterThan(-1)
expect(ethIndex).toBeGreaterThan(-1)
expect(usdcIndex).toBeGreaterThan(-1)

expect(xlmIndex).toBeLessThan(btcIndex)
expect(btcIndex).toBeLessThan(usdcIndex)
expect(usdcIndex).toBeLessThan(ethIndex)

expect(tooltip).toHaveTextContent(/Total absolute drift/)
expect(tooltip.textContent).toContain('XLM')
expect(tooltip.textContent).toContain('+5.0%')
})

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win

Assert the calculated contribution percentages.

This verifies order but not the new percentage calculation. With this fixture, assert XLM’s 42% contribution and the 12.0% total so regressions in the breakdown formula fail.

🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@frontend/src/components/DriftGauge.test.tsx` around lines 83 - 110, Update
the test for DriftGaugeGrid’s title-hover tooltip to assert the calculated
contribution values in addition to ordering: verify XLM displays a 42%
contribution and the breakdown shows a 12.0% total, while preserving the
existing asset-order and drift assertions.

Comment on lines +230 to +240
const [showBreakdown, setShowBreakdown] = useState(false)

if (assets.length === 0) return null

const sortedAssets = useMemo(() => {
return [...assets].sort((a, b) => Math.abs(b.current - b.target) - Math.abs(a.current - a.target))
}, [assets])

const totalDrift = useMemo(() => {
return assets.reduce((sum, a) => sum + Math.abs(a.current - a.target), 0)
}, [assets])

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🩺 Stability & Availability | 🟠 Major | ⚡ Quick win

🧩 Analysis chain

🏁 Script executed:

#!/bin/bash
# Inspect the conditional return and all hooks in this component.
nl -ba frontend/src/components/DriftGauge.tsx | sed -n '226,242p'

# Check whether tests cover an empty-to-populated rerender transition.
rg -n -C2 'rerender|assets=\{\[\]\}|DriftGaugeGrid' frontend/src/components/DriftGauge.test.tsx

Repository: ritik4ever/stellar-portfolio-rebalancer

Length of output: 1415


🏁 Script executed:

#!/bin/bash
set -eu

echo "== Component outline/matches =="
rg -n "function DriftGaugeGrid|const DriftGaugeGrid|useState|useMemo|useEffect|useRef|assets\.length === 0|showBreakdown" frontend/src/components/DriftGauge.tsx

echo
echo "== DriftGaugeGrid body =="
python3 - <<'PY'
from pathlib import Path
p=Path("frontend/src/components/DriftGauge.tsx")
lines=p.read_text().splitlines()
for i in range(200, 260):
    if i <= len(lines):
        print(f"{i:4d}: {lines[i-1]}")
PY

echo
echo "== Test file outline/matches =="
rg -n "DriftGaugeGrid|rerender|assets=\{\[\]\}|assets=.*sampleAssets|empty" frontend/src/components/DriftGauge.test.tsx -C 2

Repository: ritik4ever/stellar-portfolio-rebalancer

Length of output: 4392


🌐 Web query:

React Rules of Hooks keep hooks out of early or conditional return React throw

💡 Result:

React prohibits calling hooks after an early return because the library relies on a stable, consistent order of hook calls across every render [1][2]. When a component renders, React tracks the state of hooks by maintaining them in a fixed sequence—often visualized as a linked list [3]. If you place a hook after an early return (e.g., if a condition triggers a return before the hook is reached), that hook may be skipped in some renders but called in others [4][2]. This disrupts the order of the hook list, causing React to lose track of which state, effect, or context corresponds to which specific hook call [4][3]. If you break this rule, React will likely throw an error such as "Rendered fewer hooks than expected" [5]. This happens because React cannot distinguish between an intentional early return and an error where a hook call was accidentally placed inside a condition [5]. To fix this issue: - Always call hooks at the top level of your component or custom hook, before any conditional logic or early returns [1][6]. - If you need to perform an action conditionally, move the condition inside the hook itself (e.g., inside a useEffect) [4][2][3]. - If you need to return early from a component, ensure that all hooks are called unconditionally before that return statement [1][7]. The ESLint plugin 'eslint-plugin-react-hooks' is the standard tool for automatically detecting and preventing these violations in your codebase [1][8]. Note that the 'use' hook is an exception to these rules and can be called conditionally [4].

Citations:


Keep hooks before the empty-state return.

DriftGaugeGrid renders useState and then returns early for assets.length === 0; on a mounted rerender that returns populated assets, sortedAssets and totalDrift are added after an earlier render had fewer hooks, causing React to throw about hook order.

Proposed fix
   const [showBreakdown, setShowBreakdown] = useState(false)

-  if (assets.length === 0) return null
-
   const sortedAssets = useMemo(() => {
     return [...assets].sort((a, b) => Math.abs(b.current - b.target) - Math.abs(a.current - a.target))
   }, [assets])

   const totalDrift = useMemo(() => {
     return assets.reduce((sum, a) => sum + Math.abs(a.current - a.target), 0)
   }, [assets])
+
+  if (assets.length === 0) return null
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
const [showBreakdown, setShowBreakdown] = useState(false)
if (assets.length === 0) return null
const sortedAssets = useMemo(() => {
return [...assets].sort((a, b) => Math.abs(b.current - b.target) - Math.abs(a.current - a.target))
}, [assets])
const totalDrift = useMemo(() => {
return assets.reduce((sum, a) => sum + Math.abs(a.current - a.target), 0)
}, [assets])
const [showBreakdown, setShowBreakdown] = useState(false)
const sortedAssets = useMemo(() => {
return [...assets].sort((a, b) => Math.abs(b.current - b.target) - Math.abs(a.current - a.target))
}, [assets])
const totalDrift = useMemo(() => {
return assets.reduce((sum, a) => sum + Math.abs(a.current - a.target), 0)
}, [assets])
if (assets.length === 0) return null
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@frontend/src/components/DriftGauge.tsx` around lines 230 - 240, Move the
assets-empty check in DriftGaugeGrid below the sortedAssets and totalDrift
useMemo hooks so every render invokes hooks in the same order. Preserve
returning null for empty assets while keeping the existing memoized calculations
unchanged.

Comment on lines +254 to +265
<h3
id="drift-gauge-heading"
className="text-sm font-semibold text-gray-900 dark:text-white mb-4 cursor-default"
tabIndex={0}
>
{title}
</h3>

{showBreakdown && (
<div
role="tooltip"
className="absolute bottom-full mb-1 left-0 z-50 w-64 rounded-lg bg-white dark:bg-gray-800 border border-gray-200 dark:border-gray-700 shadow-lg p-3 text-xs pointer-events-none"

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win

🧩 Analysis chain

🏁 Script executed:

#!/bin/bash
set -euo pipefail

echo "Find DriftGauge.tsx:"
fd -a 'DriftGauge\.tsx$' . || true

echo
echo "Outline:"
if [ -f frontend/src/components/DriftGauge.tsx ]; then
  ast-grep outline frontend/src/components/DriftGauge.tsx --view expanded || true
  echo
  echo "Relevant lines 220-290:"
  sed -n '220,290p' frontend/src/components/DriftGauge.tsx | cat -n -v
fi

echo
echo "Search Aria descriptions/usages nearby:"
rg -n "aria-describedby|tooltip|drift-gauge-heading|showBreakdown|useId|role=\"tooltip\"" frontend/src/components/DriftGauge.tsx frontend/src -S || true

Repository: ritik4ever/stellar-portfolio-rebalancer

Length of output: 8798


Associate the drift tooltip with its heading.

The keyboard-focusable drift-gauge-heading controls a role="tooltip" breakdown, but there is no aria-describedby linking them. Generate a stable tooltip id with useId() and reference it from the heading so assistive tech receives the drift details.

🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@frontend/src/components/DriftGauge.tsx` around lines 254 - 265, Update the
DriftGauge component to generate a stable tooltip identifier with React useId,
assign it to the conditional breakdown element with role="tooltip", and add the
matching aria-describedby reference to the drift-gauge-heading element. Keep the
association valid when showBreakdown is false.

Comment on lines +100 to +170
const liveStepStatuses = useMemo(() => {
const statuses: Record<string, boolean> = {}
STEPS.forEach((step) => {
switch (step.id) {
case 'connect-wallet':
return !!publicKey
statuses[step.id] = !!publicKey
break
case 'create-portfolio':
return hasPortfolio
statuses[step.id] = hasPortfolio
break
case 'set-allocations':
return hasAllocations
statuses[step.id] = hasAllocations
break
case 'execute-rebalance':
return hasRebalanced
statuses[step.id] = hasRebalanced
break
case 'enable-auto-rebalance':
return hasAutoRebalance
statuses[step.id] = hasAutoRebalance
break
default:
return false
statuses[step.id] = false
}
},
[publicKey, hasPortfolio, hasAllocations, hasRebalanced, hasAutoRebalance]
)
})
return statuses
}, [publicKey, hasPortfolio, hasAllocations, hasRebalanced, hasAutoRebalance])

useEffect(() => {
if (!dismissed && !hasAutoShown && !allCompleted) {
const timer = setTimeout(() => setOpen(true), 800)
setHasAutoShown(true)
return () => clearTimeout(timer)
if (!initialLoadDone.current) {
initialLoadDone.current = true
const saved = loadCompletedSteps()
const merged = new Set(saved)
for (const [id, done] of Object.entries(liveStepStatuses)) {
if (done) merged.add(id)
}
setCompletedSteps(merged)
}
}, [liveStepStatuses])

useEffect(() => {
if (!initialLoadDone.current) return
const updated = new Set(completedSteps)
let changed = false
for (const [id, done] of Object.entries(liveStepStatuses)) {
if (done && !updated.has(id)) {
updated.add(id)
changed = true
}
}
if (changed) {
setCompletedSteps(updated)
saveCompletedSteps(updated)
}
}, [dismissed, hasAutoShown, allCompleted])
}, [liveStepStatuses])

useEffect(() => {
saveCompletedSteps(completedSteps)
}, [completedSteps])

const allCompleted = useMemo(() => STEPS.every((s) => completedSteps.has(s.id)), [completedSteps])

const stepStatus = useCallback(
(id: string) => completedSteps.has(id),
[completedSteps]
)

useEffect(() => {
if (dismissed || hasAutoShownRef.current || allCompleted) return
hasAutoShownRef.current = true
const timer = setTimeout(() => setOpen(true), 800)
return () => clearTimeout(timer)
}, [dismissed, allCompleted])

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🎯 Functional Correctness | 🟠 Major | ⚡ Quick win

Reset doesn't stick: unrelated live-status changes resurrect already-reset steps.

liveStepStatuses (Lines 100-124) is a brand-new object whenever any of its inputs (publicKey, hasPortfolio, hasAllocations, hasRebalanced, hasAutoRebalance) changes. The sync effect at Lines 138-152 re-runs on every such change and iterates all entries, re-adding any step that is still live-true and not yet in completedSteps — it doesn't distinguish "just became true" from "was already true before (and possibly reset)". So: user hits Reset (clears completedSteps), but if any other live condition changes afterward (e.g., a rebalance runs, allocations change) while, say, publicKey is still set, connect-wallet — which is still live-true — gets silently re-added, undoing part of the reset the user didn't ask to undo.

Separately, effect1 (Lines 126-136) and effect2 (Lines 138-152) both fire in the same initial commit (the ref guard flips synchronously), performing equivalent merges from the same source data — harmless but redundant duplicate work/persist on every mount.

Also note effect2 reads completedSteps from closure without listing it as a dependency (react-hooks/exhaustive-deps).

Suggest making the live→persisted sync edge-triggered (only add a step when it transitions from not-true to true), which also naturally merges the two effects into one:

♻️ Proposed consolidated, edge-triggered sync
-  useEffect(() => {
-    if (!initialLoadDone.current) {
-      initialLoadDone.current = true
-      const saved = loadCompletedSteps()
-      const merged = new Set(saved)
-      for (const [id, done] of Object.entries(liveStepStatuses)) {
-        if (done) merged.add(id)
-      }
-      setCompletedSteps(merged)
-    }
-  }, [liveStepStatuses])
-
-  useEffect(() => {
-    if (!initialLoadDone.current) return
-    const updated = new Set(completedSteps)
-    let changed = false
-    for (const [id, done] of Object.entries(liveStepStatuses)) {
-      if (done && !updated.has(id)) {
-        updated.add(id)
-        changed = true
-      }
-    }
-    if (changed) {
-      setCompletedSteps(updated)
-      saveCompletedSteps(updated)
-    }
-  }, [liveStepStatuses])
+  const prevLiveStatusesRef = useRef<Record<string, boolean> | null>(null)
+
+  useEffect(() => {
+    const prev = prevLiveStatusesRef.current
+    prevLiveStatusesRef.current = liveStepStatuses
+
+    if (!initialLoadDone.current) {
+      initialLoadDone.current = true
+      const merged = new Set(loadCompletedSteps())
+      for (const [id, done] of Object.entries(liveStepStatuses)) {
+        if (done) merged.add(id)
+      }
+      setCompletedSteps(merged)
+      return
+    }
+
+    setCompletedSteps((current) => {
+      let changed = false
+      const updated = new Set(current)
+      for (const [id, done] of Object.entries(liveStepStatuses)) {
+        const wasTrue = prev?.[id] === true
+        if (done && !wasTrue && !updated.has(id)) {
+          updated.add(id)
+          changed = true
+        }
+      }
+      return changed ? updated : current
+    })
+  }, [liveStepStatuses])

Worth adding a regression test with a non-null publicKey (or another live-true condition) to cover the reset-then-unrelated-status-change scenario.

📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
const liveStepStatuses = useMemo(() => {
const statuses: Record<string, boolean> = {}
STEPS.forEach((step) => {
switch (step.id) {
case 'connect-wallet':
return !!publicKey
statuses[step.id] = !!publicKey
break
case 'create-portfolio':
return hasPortfolio
statuses[step.id] = hasPortfolio
break
case 'set-allocations':
return hasAllocations
statuses[step.id] = hasAllocations
break
case 'execute-rebalance':
return hasRebalanced
statuses[step.id] = hasRebalanced
break
case 'enable-auto-rebalance':
return hasAutoRebalance
statuses[step.id] = hasAutoRebalance
break
default:
return false
statuses[step.id] = false
}
},
[publicKey, hasPortfolio, hasAllocations, hasRebalanced, hasAutoRebalance]
)
})
return statuses
}, [publicKey, hasPortfolio, hasAllocations, hasRebalanced, hasAutoRebalance])
useEffect(() => {
if (!dismissed && !hasAutoShown && !allCompleted) {
const timer = setTimeout(() => setOpen(true), 800)
setHasAutoShown(true)
return () => clearTimeout(timer)
if (!initialLoadDone.current) {
initialLoadDone.current = true
const saved = loadCompletedSteps()
const merged = new Set(saved)
for (const [id, done] of Object.entries(liveStepStatuses)) {
if (done) merged.add(id)
}
setCompletedSteps(merged)
}
}, [liveStepStatuses])
useEffect(() => {
if (!initialLoadDone.current) return
const updated = new Set(completedSteps)
let changed = false
for (const [id, done] of Object.entries(liveStepStatuses)) {
if (done && !updated.has(id)) {
updated.add(id)
changed = true
}
}
if (changed) {
setCompletedSteps(updated)
saveCompletedSteps(updated)
}
}, [dismissed, hasAutoShown, allCompleted])
}, [liveStepStatuses])
useEffect(() => {
saveCompletedSteps(completedSteps)
}, [completedSteps])
const allCompleted = useMemo(() => STEPS.every((s) => completedSteps.has(s.id)), [completedSteps])
const stepStatus = useCallback(
(id: string) => completedSteps.has(id),
[completedSteps]
)
useEffect(() => {
if (dismissed || hasAutoShownRef.current || allCompleted) return
hasAutoShownRef.current = true
const timer = setTimeout(() => setOpen(true), 800)
return () => clearTimeout(timer)
}, [dismissed, allCompleted])
const liveStepStatuses = useMemo(() => {
const statuses: Record<string, boolean> = {}
STEPS.forEach((step) => {
switch (step.id) {
case 'connect-wallet':
statuses[step.id] = !!publicKey
break
case 'create-portfolio':
statuses[step.id] = hasPortfolio
break
case 'set-allocations':
statuses[step.id] = hasAllocations
break
case 'execute-rebalance':
statuses[step.id] = hasRebalanced
break
case 'enable-auto-rebalance':
statuses[step.id] = hasAutoRebalance
break
default:
statuses[step.id] = false
}
})
return statuses
}, [publicKey, hasPortfolio, hasAllocations, hasRebalanced, hasAutoRebalance])
const prevLiveStatusesRef = useRef<Record<string, boolean> | null>(null)
useEffect(() => {
const prev = prevLiveStatusesRef.current
prevLiveStatusesRef.current = liveStepStatuses
if (!initialLoadDone.current) {
initialLoadDone.current = true
const merged = new Set(loadCompletedSteps())
for (const [id, done] of Object.entries(liveStepStatuses)) {
if (done) merged.add(id)
}
setCompletedSteps(merged)
return
}
setCompletedSteps((current) => {
let changed = false
const updated = new Set(current)
for (const [id, done] of Object.entries(liveStepStatuses)) {
const wasTrue = prev?.[id] === true
if (done && !wasTrue && !updated.has(id)) {
updated.add(id)
changed = true
}
}
return changed ? updated : current
})
}, [liveStepStatuses])
useEffect(() => {
saveCompletedSteps(completedSteps)
}, [completedSteps])
const allCompleted = useMemo(() => STEPS.every((s) => completedSteps.has(s.id)), [completedSteps])
const stepStatus = useCallback(
(id: string) => completedSteps.has(id),
[completedSteps]
)
useEffect(() => {
if (dismissed || hasAutoShownRef.current || allCompleted) return
hasAutoShownRef.current = true
const timer = setTimeout(() => setOpen(true), 800)
return () => clearTimeout(timer)
}, [dismissed, allCompleted])
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@frontend/src/components/OnboardingChecklist.tsx` around lines 100 - 170,
Update the liveStepStatuses synchronization in OnboardingChecklist so persisted
steps are added only when a live status transitions from false to true,
preventing unrelated status changes from resurrecting steps cleared by Reset.
Consolidate the initial-load merge and ongoing sync into one effect, track
previous live statuses, and include all referenced state in dependencies without
relying on a stale completedSteps closure. Add a regression test covering reset
followed by an unrelated live-status change while another condition remains
true.

Comment on lines 57 to +69
const POLL_MS = 45_000
const HIDDEN_POLL_MS = POLL_MS * 4

/**
* React Query version of useReadinessReport.
* Replaces manual setInterval polling with TanStack Query refetchInterval.
* Polling frequency adapts based on tab visibility (backoff when hidden).
*/
export function useReadinessQuery() {
const { data: report, isLoading: loading, isError: loadError, refetch: refresh } = useQuery({
queryKey: readinessKeys.all,
queryFn: fetchReadinessReport,
refetchInterval: POLL_MS,
refetchInterval: () => document.visibilityState === 'visible' ? POLL_MS : HIDDEN_POLL_MS,

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🎯 Functional Correctness | 🟠 Major | ⚡ Quick win

🧩 Analysis chain

🏁 Script executed:

#!/bin/bash
set -euo pipefail
rg -n -C 3 'refetchInterval(InBackground)?|visibilitychange|useReadinessQuery' .

Repository: ritik4ever/stellar-portfolio-rebalancer

Length of output: 10524


🏁 Script executed:

#!/bin/bash
set -euo pipefail

echo "## useReadinessQuery.ts"
cat -n frontend/src/hooks/queries/useReadinessQuery.ts | sed -n '1,120p'

echo
echo "## TanStack Query dependency versions"
for f in package.json frontend/package.json pnpm-lock.yaml package-lock.json yarn.lock; do
  if [ -f "$f" ]; then
    echo "--- $f ---"
    rg -n '`@tanstack`|useQuery|query-key' "$f" | head -80 || true
  fi
done

echo
echo "## Nearby old visibility polling implementation"
cat -n frontend/src/hooks/useReadinessReport.ts | sed -n '160,210p'

Repository: ritik4ever/stellar-portfolio-rebalancer

Length of output: 6022


🌐 Web query:

TanStack Query v5 refetchIntervalInBackground default pause interval fetches in background tabs

💡 Result:

In TanStack Query v5, polling is paused by default when the browser tab loses focus [1][2]. The refetchIntervalInBackground option is used to override this behavior; when set to true, queries configured with a refetchInterval will continue to refetch even while the browser tab is inactive in the background [3][4]. However, it is important to note that this setting specifically governs interval-based refetching. Other automatic background processes, such as query retries, may still pause when the tab is inactive because they are tied to window focus events independently of the refetchIntervalInBackground configuration [5][6]. Summary of behavior: - Default: Polling pauses when the tab loses focus [1][2]. - With refetchIntervalInBackground: true: Polling continues while the tab is in the background [3][4]. - Note on Retries: Retries currently remain subject to tab focus behavior, meaning they may pause in the background even if refetchIntervalInBackground is enabled for your periodic polling [5][6].

Citations:


Make visibility changes actually affect the hidden polling interval.

With TanStack Query’s default behavior, interval refetches are paused when the tab loses focus, so the HIDDEN_POLL_MS slowdown is not reached; if this hidden-rate behavior is needed, pass refetchIntervalInBackground: true. Also, reading document.visibilityState directly in refetchInterval does not subscribe the query to visibilitychange, so the current interval may not switch on visibility changes. Use a React-visible isVisible state updated from a SSR-safe visibilitychange listener and pass that into refetchInterval.

🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@frontend/src/hooks/queries/useReadinessQuery.ts` around lines 57 - 69, The
useReadinessQuery polling interval does not reliably adapt to tab visibility
changes. Add SSR-safe React visibility state updated by a visibilitychange
listener, use that isVisible state in refetchInterval to select POLL_MS or
HIDDEN_POLL_MS, and enable refetchIntervalInBackground so hidden-tab polling
continues at the slower rate.

Source: MCP tools

@ritik4ever
ritik4ever merged commit 478e822 into ritik4ever:main Jul 30, 2026
8 of 27 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

2 participants